Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix size of embed loader, hide search on sm devices and improve theme update #278

Merged
merged 5 commits into from
Feb 25, 2025

Conversation

Rodri77
Copy link
Collaborator

@Rodri77 Rodri77 commented Feb 24, 2025

Important

Improved UI with theme toggle, adjusted embed loader size, and hid search on small devices, along with minor environment and package updates.

  • UI Improvements:
    • page.tsx: Added theme toggle button with light/dark mode functionality.
    • embed-react.tsx: Adjusted spinner size to 30px and set max-width/height for iframe.
    • IntegrationSearch.tsx: Hid search bar on small devices when less than 8 integrations.
  • Environment Variables:
    • Changed OPENINT_TOKEN to NEXT_PUBLIC_OPENINT_TOKEN in .env.example.
  • Package Updates:
    • Updated version in package.json from 0.2.18 to 0.2.19.
  • Theme Handling:
    • ConnectionPortal.tsx: Removed redundant theme check before setting theme.

This description was created by Ellipsis for 3baf10e. It will automatically update as commits are pushed.

@Rodri77 Rodri77 requested a review from pellicceama February 24, 2025 23:17
Copy link

vercel bot commented Feb 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
openint ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 0:15am

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 4530d9a in 1 minute and 41 seconds

More details
  • Looked at 161 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 13 drafted comments based on config settings.
1. apps/connect-dev/.env.example:1
  • Draft comment:
    Rename token variable to public env var. This change exposes the token in the browser as intended.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. kits/connect/src/embed-react.tsx:45
  • Draft comment:
    Use DEFAULT_WIDTH as fallback for iframe width for consistency. Spinner container now also constrains dimensions.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
3. packages/engine-frontend/components/ConnectionPortal.tsx:56
  • Draft comment:
    Removed destructured 'theme' from useTheme. Ensure that dark mode styling is handled by a parent component if needed, since the conditional dark class has been removed.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
4. packages/engine-frontend/components/IntegrationSearch.tsx:134
  • Draft comment:
    Hide search header on small devices when integrations are few. The logic conditionally adds 'hidden md:block'.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, describing what the code does without suggesting any changes or asking for clarification. It doesn't provide any actionable feedback or raise any concerns about the code.
5. apps/connect-dev/.env.example:1
  • Draft comment:
    Changed env var name to NEXT_PUBLIC_OPENINT_TOKEN for proper public exposure.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, stating that an environment variable name was changed for public exposure. It doesn't provide a suggestion, ask for confirmation, or highlight a potential issue.
6. apps/connect-dev/app/page.tsx:41
  • Draft comment:
    Consider extracting inline mouse event styles (onMouseOver/onMouseOut) into CSS classes for maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
7. kits/connect/src/embed-react.tsx:47
  • Draft comment:
    Using DEFAULT_WIDTH instead of '100%' ensures consistent sizing; confirm responsiveness meets design needs.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
8. kits/connect/src/embed-react.tsx:96
  • Draft comment:
    Added max-width and max-height to spinner container to enforce fixed embed dimensions.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, describing what was done without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
9. kits/connect/src/embed-react.tsx:105
  • Draft comment:
    Reduced spinner size from 50px to 30px for a more compact embed loader.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, describing a change that was made without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
10. packages/engine-frontend/components/ConnectionPortal.tsx:56
  • Draft comment:
    Removed 'theme' from useTheme destructuring; ensure dark mode styling is managed appropriately via the provider.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to ensure that dark mode styling is managed appropriately, which falls under the category of asking the author to ensure behavior is intended or tested. This violates the rules.
11. packages/engine-frontend/components/ConnectionPortal.tsx:66
  • Draft comment:
    Simplified theme update logic by removing the redundant check against the current theme; verify this aligns with expected behavior.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify if the change aligns with expected behavior, which violates the rule against asking for confirmation of intention or behavior. It does not provide a specific code suggestion or ask for a specific test to be written.
12. packages/engine-frontend/components/ConnectionPortal.tsx:159
  • Draft comment:
    Removed manual dark class toggle from container; ensure dark mode styles are applied via the theme provider.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
13. packages/engine-frontend/components/IntegrationSearch.tsx:134
  • Draft comment:
    Conditional class hides search header on small devices when integrations count is below 8; confirm this meets UX requirements.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking for confirmation on whether the behavior meets UX requirements, which is against the rules. It doesn't provide a specific code suggestion or point out a clear issue with the code.

Workflow ID: wflow_7cweeBIYkWkTTHQu


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

import {OpenIntConnectEmbed} from '../../../kits/connect/src/embed-react'

export default function Home() {
const [theme, setTheme] = useState<'light' | 'dark'>('light')
const token = process.env['NEXT_PUBLIC_OPENINT_TOKEN']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No error handling or validation for missing environment variable. Should check if token exists and handle the undefined case to avoid runtime errors.


💬 Reply with /praise to tell me that this comment was useful.

Is this comment irrelevant to your project? Reply with /ignore to no longer receive comments like this in the future.

Copy link

recurseml bot commented Feb 24, 2025

😱 Found 1 issue. Time to roll up your sleeves! 😱

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 3baf10e in 38 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. kits/connect/package.json:3
  • Draft comment:
    Version bump update is valid. Ensure corresponding release notes are updated.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that release notes are updated, which falls under the category of asking the author to ensure something is done. This violates the rule against asking the author to ensure things are done.
2. kits/connect/package.json:3
  • Draft comment:
    Ensure the semantic version bump is appropriate for the changes. Bumping from 0.2.18 to 0.2.19 seems correct if these are patch fixes.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to verify the semantic version bump, which is not allowed as it falls under asking the author to confirm or verify something. The comment does not provide a specific suggestion or point out a clear issue with the version bump, making it not useful according to the rules.

Workflow ID: wflow_0f7lkrmcemnYgf0v


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@pellicceama pellicceama merged commit 6db2024 into main Feb 25, 2025
4 checks passed
@pellicceama pellicceama deleted the embed-loader branch February 25, 2025 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants